Connect Kube gateway part 4: frontend by greedy52 · Pull Request #29376 · gravitational/teleport

greedy52 · 2023-07-20T20:34:01Z

Related PR:

greedy52 · 2023-07-20T20:37:53Z

Note that this change does not include migrating from old tsh kube terminal to new terminal.

And since my first time doing these change, not sure how much testing/story I should add.

@ravicious @gzdunek one usage thing, when the kube terminal is open, there is no indication of what user should do. KUBECONFIG=<xxx> is automatically set but there is no instructions. Maybe somehow we can print a message in the terminal?

greedy52 · 2023-07-20T20:39:31Z

+    case 'success': {
+      return <DocumentTerminal doc={doc} visible={visible} />;
+    }
+
+    case 'error': {
+      return (
+        <Reconnect
+          kubeId={params.kubeId}
+          statusText={connectAttempt.statusText}
+          reconnect={createGateway}
+        />
+      );
+    }


not sure how these look. happy to improve them if anyone has ideas.

Looks good. 👌

ravicious · 2023-07-21T15:24:12Z

I won't have the time to review this today, but I still wanted to offer some help.

one usage thing, when the kube terminal is open, there is no indication of what user should do. KUBECONFIG=<xxx> is automatically set but there is no instructions. Maybe somehow we can print a message in the terminal?

We should be able to do it. We used to execute arbitrary commands by writing to the PTY which was not the best idea. But, instead of writing to PTY, we can make Xterm.js think that the PTY emitted some data, even though it's just us and not the actual PTY.

This works:

diff --git a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts
index 9814d52ff5..5cf443234f 100644
--- a/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts
+++ b/web/packages/teleterm/src/sharedProcess/ptyHost/ptyProcess.ts
@@ -76,6 +76,11 @@ export class PtyProcess extends EventEmitter implements IPtyProcess {
     this._setStatus('open');
     this.emit(TermEventEnum.Open);
 
+    this.emit(
+      TermEventEnum.Data,
+      'hey hello \r\nsome fake data is being emitted here \r\n'
+    );
+
     this._process.onData(data => this._handleData(data));
     this._process.onExit(ev => this._handleExit(ev));
   }

And since we emit before we add the onData listener, we're guaranteed that it won't conflict with data actually emitted by the PTY.

Look how initCommand was used (#26837). We could instead have an option such as initMessage and then pass it only for those new kube terminals.

The message could be even the same as tsh kube login outputs currently.

…/26836_connect_kube_gateway_go_part_41

greedy52 · 2023-07-24T14:28:51Z

We should be able to do it. We used to execute arbitrary commands by printing to the PTY which was not the best idea. But, instead of writing to PTY, we can make Xterm.js think that the PTY emitted some data, even though it's just us and not the actual PTY.

Thanks! I tried this today and added a helpmsg to the pty option. It currently looks something like this. Let me know what you think.

ravicious

I reviewed as much as I could today, I'll continue the review tomorrow.

ravicious · 2023-07-25T13:26:17Z

+  rootClusterId: string;
+  leafClusterId: string | undefined;


You don't need these two fields for this document type AFAIK.

I got an error here when removing these 😭

teleport/web/packages/teleterm/src/ui/DocumentTerminal/useDocumentTerminal.ts

Lines 219 to 238 in e444901

const clusterUri = routing.getClusterUri({

rootClusterId: doc.rootClusterId,

leafClusterId: doc.leafClusterId,

});

const rootCluster = ctx.clustersService.findRootClusterByResource(clusterUri);

const cmd = createCmd(

ctx.clustersService,

doc,

rootCluster.proxyHost,

getClusterName()

);

const ptyProcess = await createPtyProcess(ctx, cmd);

if (doc.kind === 'doc.terminal_tsh_node') {

ctx.usageService.captureProtocolUse(clusterUri, 'ssh', doc.origin);

}

if (doc.kind === 'doc.terminal_tsh_kube') {

ctx.usageService.captureProtocolUse(clusterUri, 'kube', doc.origin);

}

Ah, I forgot about that. 🤦 Let's just add the same comment above them to say they're tech debt, DocumentGatewayCliClient already has a comment like that.

ravicious · 2023-07-25T13:28:04Z

Could you summarize the next steps regarding the "migration" to the new document kind, based on the discussion we had in one of the previous PRs?

Edit: nvm, I think it's just this? #28312 (comment)

ravicious

I tested it with the old kube tabs and everything works as expected. Overall I really like how this works and I'm glad we didn't run into any unexpected roadblocks, which means we planned everything pretty well.

I also tested this on Windows, no problems there.

The only thing we should add is more deprecation comments, maybe linking to an issue where we describe what the next steps are to move away from the previous implementation of kube tabs, based on the discussion from #28312 (comment). Could you take care of that?

Helper function to open old kube tab

Put this in connectToKube.ts, then restart the app, open console in devtools in Electron and execute the function with the URI of the kube, which you can inspect also in devtools when opening the table with kubes.

window['connectToOldKube'] = async uri => {
  const ctx: IAppContext = window['teleterm'];
  const rootClusterUri = routing.ensureRootClusterUri(uri);
  const documentsService =
    ctx.workspacesService.getWorkspaceDocumentService(rootClusterUri);
  const kubeDoc = documentsService.createTshKubeDocument({
    kubeUri: uri,
    origin: 'resource_table',
  });
  const connection = ctx.connectionTracker.findConnectionByDocument(kubeDoc);

  await ctx.workspacesService.setActiveWorkspace(rootClusterUri);

  documentsService.add({
    ...kubeDoc,
    kubeConfigRelativePath:
      (connection && connection['kubeConfigRelativePath']) ||
      kubeDoc.kubeConfigRelativePath,
  });
  documentsService.open(kubeDoc.uri);
};

ravicious · 2023-07-26T13:15:30Z

The docs need to be updated to mention that running Connect is now required for the KUBECONFIG to work. I suppose it'd be best to do it in a separate PR.

teleport/docs/pages/connect-your-client/teleport-connect.mdx

Lines 92 to 104 in b3d123e

## Connecting to a Kubernetes cluster

1. Open a tab with cluster resources by clicking on the plus symbol at the right end of the tab bar.

You can also press <span style="white-space: nowrap;">`Ctrl/Cmd + T`</span> to achieve the same result.

1. Select the Kubes section.

1. Look for the cluster you wish to connect to and click the Connect button to the right.

A new local terminal tab will open which is preconfigured with the `$KUBECONFIG` environment variable

pointing to a configuration for the specified cluster. Any tools that you have installed that respect

the `$KUBECONFIG` environment variable (`kubectl`, `helm`, etc.) will work without additional configuration.

To identify the path to this config for use in other tools, run `echo $KUBECONFIG`.

Alternatively, you can look for the cluster in the search bar and press `Enter` to connect to it.

sounds good. i will do a separate doc change on this. And likely need to hold off merging the doc change until this is released.

ravicious · 2023-07-26T13:29:42Z

 export interface TrackedKubeConnection extends TrackedConnectionBase {
  kind: 'connection.kube';
-  kubeConfigRelativePath: string;
+  kubeConfigRelativePath?: string;


Suggested change

kubeConfigRelativePath?: string;

/**

* @deprecated Used only by connections created by doc.terminal_tsh_kube.

*/

kubeConfigRelativePath?: string;

ravicious · 2023-07-26T14:29:22Z

And since my first time doing these change, not sure how much testing/story I should add.

As far as frontend tests are involved, I think you added an adequate amount of them. The terminal tabs don't really have proper integration tests anyway and I wouldn't expect you to set all of this up.

greedy52 · 2023-07-26T20:10:02Z

Thanks @ravicious for amazing review/comments!

Helper function to open old kube tab

I didn't get a chance to try this out. will give a devtools a try later.

The only thing we should add is more deprecation comments

I have added some comments for removing the old implementation in 30bb201. I followed the way it's done in go but is there a standard how to mark deprecate in js? Also, most likely I have not covered everything that needs to be deleted later.

gzdunek · 2023-07-27T07:17:07Z

+  //
+  // The helpMessage is rendered on the terminal UI without being written or
+  // read by the underlying PTY.
+  helpMessage?: string;


In the other place we have initMessage, I'd use it everywhere.

gzdunek · 2023-07-27T07:26:48Z

 });

-export const makeGateway = (props: Partial<tsh.Gateway> = {}): tsh.Gateway => ({
+export const makeDBGateway = (


Nit: I'd call it makeDatabaseGateway (to align with, for example, DatabaseUri).

ravicious · 2023-07-27T13:39:52Z

I have added some comments for removing the old implementation in 30bb201. I followed the way it's done in go but is there a standard how to mark deprecate in js? Also, most likely I have not covered everything that needs to be deleted later.

The comment you left should be enough. There's jsdoc's @deprecated which I added in a03a662 to a single function – if someone has their editor set up to integrate with the lang server, it should highlight the function at the callsites as deprecated. But those DELETE IN comments should be enough everywhere else.

…/26836_connect_kube_gateway_go_part_41

Connect Kube gateway part 4: frontend

c527bf2

greedy52 added kubernetes-access tls-routing Issues related to TLS routing teleport-connect Issues related to Teleport Connect. labels Jul 20, 2023

greedy52 requested review from gzdunek and ravicious July 20, 2023 20:34

greedy52 self-assigned this Jul 20, 2023

greedy52 requested a review from Joerger July 20, 2023 20:34

github-actions Bot added size/md ui labels Jul 20, 2023

github-actions Bot requested a review from kimlisa July 20, 2023 20:34

greedy52 commented Jul 20, 2023

View reviewed changes

Comment thread ...m/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx Outdated

greedy52 added 2 commits July 24, 2023 09:00

Merge branch 'master' of github.com:gravitational/teleport into STeve…

fab1ce1

…/26836_connect_kube_gateway_go_part_41

add help msg to terminal

12178db

try to fix eslint

95baf30

ravicious removed the request for review from kimlisa July 25, 2023 12:48

ravicious reviewed Jul 25, 2023

View reviewed changes

ravicious self-requested a review July 25, 2023 13:28

ClusterLogin: Handle empty targetUser in gateway

0e8a8d3

ravicious reviewed Jul 26, 2023

View reviewed changes

greedy52 added 4 commits July 26, 2023 16:02

make fix-license

2c95865

add check to test gateway.Kube.CLICommand

0c8bc8a

code review comments

c6772dc

add deprecation comments

30bb201

greedy52 requested a review from ravicious July 26, 2023 20:13

gzdunek reviewed Jul 27, 2023

View reviewed changes

ravicious added 2 commits July 27, 2023 15:33

sendNotification: Use targetUri as a fallback

5694b0d

Add deprecated commont to createTshKubeDocument

a03a662

Move updating doc status inside useAsync

6687321

ravicious approved these changes Jul 27, 2023

View reviewed changes

greedy52 added 2 commits July 27, 2023 14:43

review comments

9fbe14f

Merge branch 'master' of github.com:gravitational/teleport into STeve…

8dc2e4e

…/26836_connect_kube_gateway_go_part_41

gzdunek approved these changes Jul 28, 2023

View reviewed changes

public-teleport-github-review-bot Bot removed the request for review from Joerger July 28, 2023 10:27

greedy52 added this pull request to the merge queue Jul 28, 2023

Merged via the queue into master with commit b94a710 Jul 28, 2023

greedy52 deleted the STeve/26836_connect_kube_gateway_go_part_41 branch July 28, 2023 12:46

greedy52 mentioned this pull request Aug 14, 2023

[docs] Teleport Connect Kube gateway v14 update #30460

Merged

ravicious mentioned this pull request Jan 12, 2024

Connect: Refactor kube tabs not to use tsh kube login #22535

Closed

	const clusterUri = routing.getClusterUri({
	rootClusterId: doc.rootClusterId,
	leafClusterId: doc.leafClusterId,
	});
	const rootCluster = ctx.clustersService.findRootClusterByResource(clusterUri);
	const cmd = createCmd(
	ctx.clustersService,
	doc,
	rootCluster.proxyHost,
	getClusterName()
	);

	const ptyProcess = await createPtyProcess(ctx, cmd);

	if (doc.kind === 'doc.terminal_tsh_node') {
	ctx.usageService.captureProtocolUse(clusterUri, 'ssh', doc.origin);
	}
	if (doc.kind === 'doc.terminal_tsh_kube') {
	ctx.usageService.captureProtocolUse(clusterUri, 'kube', doc.origin);
	}

	## Connecting to a Kubernetes cluster

	1. Open a tab with cluster resources by clicking on the plus symbol at the right end of the tab bar.
	You can also press <span style="white-space: nowrap;">`Ctrl/Cmd + T`</span> to achieve the same result.
	1. Select the Kubes section.
	1. Look for the cluster you wish to connect to and click the Connect button to the right.

	A new local terminal tab will open which is preconfigured with the `$KUBECONFIG` environment variable
	pointing to a configuration for the specified cluster. Any tools that you have installed that respect
	the `$KUBECONFIG` environment variable (`kubectl`, `helm`, etc.) will work without additional configuration.
	To identify the path to this config for use in other tools, run `echo $KUBECONFIG`.

	Alternatively, you can look for the cluster in the search bar and press `Enter` to connect to it.

Conversation

greedy52 commented Jul 20, 2023

Uh oh!

greedy52 commented Jul 20, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ravicious commented Jul 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greedy52 commented Jul 24, 2023

Uh oh!

ravicious left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greedy52 Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ravicious Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ravicious left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ravicious commented Jul 26, 2023

Uh oh!

greedy52 commented Jul 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ravicious commented Jul 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ravicious commented Jul 21, 2023 •

edited

Loading

greedy52 Jul 26, 2023 •

edited

Loading

ravicious Jul 25, 2023 •

edited

Loading

greedy52 commented Jul 26, 2023 •

edited

Loading